Migrate Python tooling to UV and Ty#590
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #590 +/- ##
==========================================
- Coverage 90.11% 90.09% -0.03%
==========================================
Files 55 55
Lines 2498 2503 +5
==========================================
+ Hits 2251 2255 +4
- Misses 247 248 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hello @frgfm , Thanks for this PR. I was planning to switch from Poetry to uv, so this is perfect, thank you. I really like the idea of moving to Ty, and the Astral team is doing great work. Do you have any concerns about the project still being relatively young? Could you also explain the idea behind this new backend CI workflow, and why not reuse the existing ones? |
None at all, I had the same concerns a while back and since I migrated all my other projects, I feel like Ty is way more mature and serious than mypy (performance wise but also quality wise, remember all those mypy ignore). The nice part is that it's so fast, you can use it as a language server and pre commit hook (if you had done that with mypy, you would be waiting 30sec every time)
Yup I need to clear up some CI job redundancies actually! |
|
Hi @frgfm ok thanks let's go then ask me the review when yo uare done with cleaning 😄 |
|
Ready for review!
Feedback is very much welcome! |
MateoLostanlen
left a comment
There was a problem hiding this comment.
Hi @frgfm . the approach looks sound
A couple of things I’d fix before merge:
src/Dockerfile.testhealthcheck points to/health, but the app exposes/status. This image would report unhealthy even when the server is runningclient/.conda/meta.yamlis stale versusclient/pyproject.toml: the client now requires Python>=3.10,<4andrequests>=2.33.0,<3.0.0, while the conda recipe still advertises Python>=3.8andrequests>=2.31.0- Fix the header fleged by CI
| COPY src/alembic.ini /app/alembic.ini | ||
|
|
||
| EXPOSE 8000 | ||
| HEALTHCHECK --interval=10s --timeout=3s --retries=5 CMD ["curl", "-f", "http://localhost:8000/health", "--max-time", "3"] |
There was a problem hiding this comment.
our api exposes /status not health
fixed!
not the scope of this PR (my opinion is to remove conda all together, since uv brought first class Pytorch builds, no need for conda anymore)
fixed! |
MateoLostanlen
left a comment
There was a problem hiding this comment.
Hi ,
Thanks for 1 and 3
for 2 it's kind of the scop you created the issue here by update requests>=2.33.0. But yes agree let's drop conda in an upcoming pr
Summary
poetry.lockwith a UV workspace and a single rootuv.lock.uv sync/uv runinstead of Poetry exports.Main choices
tool.uv.package = false;pyroclientremains the only publishable package and is included as a workspace member.serverdependency group rather than rootproject.dependencies, so Docker/CI can install exact slices from the lockfile.unresolved-import = "warn", then fixed or locally suppressed migration-specific diagnostics.AsyncClientfixture to useASGITransport(app=...), matching current HTTPX syntax.docker-compose.dev.yml.KeyError: access_token.headbefore testing the downgrade/upgrade cycle.Validation
make venvuv lock --checkuv sync --locked --all-groups --all-packages --no-install-projectmake qualitymake test(481 passed)make test-client(14 passed)14 passed)AssertionError: {"detail":"Invalid credentials."}, notKeyError: access_token.github/workflows/backend.ymlcompleted with--- Migration cycle successful ---make docs-clientmake e2edocker compose -f docker-compose.dev.yml up -d --build --waitplus backend/statuscheckdocker compose -f docker-compose.dev.yml config --quietDeliberately excluded
AGENTS.mdCLAUDE.mddocker/minio/setup-s3.sh